Skip to content

Conversation

@barakcodes
Copy link
Contributor

@barakcodes barakcodes commented Dec 3, 2024

Add the regex to exclude all vite asset import suffixes, might be better done in the @hono/vite-dev-server(maybe), but this fixes the issue for now

Rel #44

@barakcodes barakcodes force-pushed the fix-vite-asset-import branch 2 times, most recently from c799af0 to cb85414 Compare December 3, 2024 20:06
@john-griffin
Copy link
Contributor

Thanks, I checked this out locally and I still see the 404 in the example you updated.

@barakcodes
Copy link
Contributor Author

Thanks, I checked this out locally and I still see the 404 in the example you updated.

Hmm, it works for me. I look more into it

@barakcodes barakcodes force-pushed the fix-vite-asset-import branch from cb85414 to 28e9b2d Compare December 4, 2024 07:17
@barakcodes
Copy link
Contributor Author

Thanks, I checked this out locally and I still see the 404 in the example you updated.

@john-griffin could you give it another try?

@john-griffin
Copy link
Contributor

Still a 404 with that pattern. I'm running via the npm run dev command if that makes any difference, not npm run start-with-adapter

@barakcodes
Copy link
Contributor Author

Still a 404 with that pattern. I'm running via the npm run dev command if that makes any difference, not npm run start-with-adapter

shoot! I'll take a deeper look at this over the weekend

Add the regix to exclude all vite asset import suffixes,
might be better done in the @hono/vite-dev-server, but this
fixes the issue for now
@barakcodes barakcodes force-pushed the fix-vite-asset-import branch from 28e9b2d to f309859 Compare December 5, 2024 17:36
@barakcodes
Copy link
Contributor Author

barakcodes commented Dec 5, 2024

So

Still a 404 with that pattern. I'm running via the npm run dev command if that makes any difference, not npm run start-with-adapter

turns out I was testing with ?inline which did not have?import prefix in some cases for some reason. this should now work as it matches both ?inline and ?import&inline and similar combinations

@john-griffin
Copy link
Contributor

Thanks, that pattern is now working, imports are being returned with a 200 👍

@barakcodes barakcodes requested a review from yusukebe December 5, 2024 18:55
@yusukebe
Copy link
Owner

yusukebe commented Dec 6, 2024

Hey @barakcodes

Thank you for fixing. I've added the test for this change.

Copy link
Owner

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe
Copy link
Owner

yusukebe commented Dec 6, 2024

Looks good! Let's land it.

@yusukebe yusukebe merged commit 3e1ee79 into yusukebe:main Dec 6, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants